Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[tempo-distributed] Service labels for distributor and query-frontend #2430

Merged
merged 2 commits into from
Jul 23, 2023

Conversation

deejgregor
Copy link
Contributor

@deejgregor deejgregor commented May 24, 2023

We have a need to add service labels to support some service mesh configuration specifically for the distributor and query-frontend. These are added following the pattern that is already used for other services that have labels for the main services.

For the existing services with user-settable labels, there aren't any that also have paired discovery services, so I wasn't sure what to do here. Looking at related annotations, there is inconsistency in how they are named across the distributor and queryFrontend:

The approach that I took was to add labels for the discovery services following the existing naming pattern followed for queryFrontend annotations. So, distributor.serviceDiscovery.labels and queryFrontend.serviceDiscovery.labels. I'm happy to rework this if you'd like it done differently (or if you'd like to avoid adding service annotations on the discovery services altogether). Of course, please feel free to edit as needed.

For reference, here are the services that previously allowed user-settable labels before this change, the ones that were added with this change, and those that do not have user-settable labels after this change. If you want them added to any other services, please let me know and I'll be happy to add them to this PR.

Previously had user-settable service labels:

  • admin-api/admin-api-svc.yaml: already has adminApi.service.labels
  • enterprise-gateway/gateway-svc.yaml: already has enterpriseGateway.service.labels
  • gateway/service-gateway.yaml: already has gateway.service.labels

Added with this PR:

  • distributor/service-distributor-discovery.yaml: adding distributor.serviceDiscovery.labels
  • distributor/service-distributor.yaml: adding distributor.service.labels
  • query-frontend/service-query-frontend-discovery.yaml: adding queryFrontend.serviceDiscovery.labels
  • query-frontend/service-query-frontend.yaml: adding queryFrontend.service.labels

No user-settable service labels yet:

  • compactor/service-compactor.yaml
  • gossip-ring/service-gossip-ring.yaml
  • ingester/service-ingester-discovery.yaml
  • ingester/service-ingester.yaml
  • memcached/service-memcached.yaml
  • metrics-generator/service-metrics-generator-discovery.yaml
  • metrics-generator/service-metrics-generator.yaml
  • querier/service-querier.yaml

Lastly, this is my first contribution to this repository, so if I need to change anything, please let me know. Thanks!

@CLAassistant
Copy link

CLAassistant commented May 24, 2023

CLA assistant check
All committers have signed the CLA.

@zanhsieh zanhsieh merged commit 2d1a7d5 into grafana:main Jul 23, 2023
@deejgregor deejgregor deleted the add-service-labels branch July 23, 2023 23:40
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants